Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix SearchableSnapshotDirectoryTests.testIndexSearcher() #56275

Merged
merged 3 commits into from
May 7, 2020
Merged

Fix SearchableSnapshotDirectoryTests.testIndexSearcher() #56275

merged 3 commits into from
May 7, 2020

Conversation

tlrx
Copy link
Member

@tlrx tlrx commented May 6, 2020 •
edited
Loading

The test SearchableSnapshotDirectoryTests.testIndexSearcher() sometimes fails with the error:

java.lang.AssertionError: current thread [Thread[LuceneTestCase-1-thread-2,5,TGRP-SearchableSnapshotDirectoryTests]] may not read [...]
    at org.elasticsearch.index.store.BaseSearchableSnapshotIndexInput.assertCurrentThreadMayAccessBlobStore(BaseSearchableSnapshotIndexInput.java:148)
    at org.elasticsearch.index.store.direct.DirectBlobContainerIndexInput.openBlobStream(DirectBlobContainerIndexInput.java:343)
    at org.elasticsearch.index.store.direct.DirectBlobContainerIndexInput.readInternalBytes(DirectBlobContainerIndexInput.java:133)
    at org.elasticsearch.index.store.direct.DirectBlobContainerIndexInput.readInternal(DirectBlobContainerIndexInput.java:105)

The method assertCurrentThreadMayAccessBlobStore() correctly checks that the thread that reads blobs is a thread created by the test framework and has the TEST- prefix in its name.

But testIndexSearcher() is slightly different from other searchable snapshots tests as it is the only one that directly opens a new IndexSearcher using LuceneTestCase's newSearcher() method which randomly opens the searcher using a testing thread pool that has a different prefix.

This commit simply adds the LuceneTestCase- prefix for testing thread.

Closes #56233

@tlrx tlrx added >test Issues or PRs that are addressing/adding tests :Distributed Coordination/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs v8.0.0 labels May 6, 2020
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed (:Distributed/Snapshot/Restore)

@elasticmachine elasticmachine added the Team:Distributed (Obsolete) Meta label for distributed team (obsolete). Replaced by Distributed Indexing/Coordination. label May 6, 2020
DaveCTurner
DaveCTurner approved these changes May 6, 2020
View reviewed changes
Copy link
Contributor

@DaveCTurner DaveCTurner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM; this is currently only labelled v8.0.0, should it be back ported to 7.8 and 7.x too?

@tlrx tlrx merged commit a443e13 into elastic:master May 7, 2020
@tlrx tlrx deleted the fix-testIndexSearcher branch May 7, 2020 09:11
tlrx added a commit that referenced this pull request May 7, 2020
@tlrx
Fix SearchableSnapshotDirectoryTests.testIndexSearcher() (#56275)
7cdd1f2 
Closes #56233
@tlrx
Copy link
Member Author

tlrx commented May 7, 2020

Thanks David

tlrx added a commit that referenced this pull request May 7, 2020
@tlrx
Fix SearchableSnapshotDirectoryTests.testIndexSearcher() (#56275)
6233e32 
Closes #56233
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DaveCTurner DaveCTurner DaveCTurner approved these changes

Assignees

@DaveCTurner DaveCTurner

Labels
:Distributed Coordination/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs Team:Distributed (Obsolete) Meta label for distributed team (obsolete). Replaced by Distributed Indexing/Coordination. >test Issues or PRs that are addressing/adding tests v7.8.0 v7.9.0 v8.0.0-alpha1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

SearchableSnapshotDirectoryTests.testIndexSearcher can fail with assertion error
4 participants
@tlrx @elasticmachine @DaveCTurner @jakelandis